Skip to content

BCM2711 YUV output through a bridge#4280

Closed
mripard wants to merge 19 commits intoraspberrypi:rpi-5.10.yfrom
mripard:rpi/5.10-hdmi-bridge-yuv
Closed

BCM2711 YUV output through a bridge#4280
mripard wants to merge 19 commits intoraspberrypi:rpi-5.10.yfrom
mripard:rpi/5.10-hdmi-bridge-yuv

Conversation

@mripard
Copy link
Copy Markdown
Contributor

@mripard mripard commented Apr 14, 2021

Hi,

Here's an alternative proposal to support the YUV output on the HDMI controllers. This has been submitted upstream and given the current discussion is likely to be abandoned in favour of something similar to #4201 but this could be useful still

mripard added 19 commits April 14, 2021 09:27
The new bridge rework to support the input and output formats introduced
some boilerplate code that will need to be shared across drivers.

Since dw-hdmi is the only driver so far, let's introduce those helpers
based on that code.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
The atomic_get_output_bus_fmts bridge callback is there to list the
available formats for output by decreasing order of preference.

On HDMI controllers, we have a fairly static list that will depend on
what the HDMI sink is capable of and the BPC our controller can output.

The dw-hdmi driver already has that code done in a fairly generic
manner, so let's turn that code into an helper for all the HDMI
controllers to reuse.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
The current code does a binary or on the possible_crtcs variable of the
TXP encoder, while we want to set it to that value instead.

Fixes: 39fcb28 ("drm/vc4: txp: Turn the TXP into a CRTC of its own")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
The vc4_set_crtc_possible_masks is meant to run over all the encoders
and then set their possible_crtcs mask to their associated pixelvalve.

However, since the commit 39fcb28 ("drm/vc4: txp: Turn the TXP into
a CRTC of its own"), the TXP has been turned to a CRTC and encoder of
its own, and while it does indeed register an encoder, it no longer has
an associated pixelvalve. The code will thus run over the TXP encoder
and set a bogus possible_crtcs mask, overriding the one set in the TXP
bind function.

In order to fix this, let's skip any virtual encoder.

Fixes: 39fcb28 ("drm/vc4: txp: Turn the TXP into a CRTC of its own")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Since we fixed the hooks to disable the encoder at boot, we now have an
unbalanced clk_disable call at boot since we never enabled them in the
first place.

Let's mimic the state of the hardware and enable the clocks at boot if
the controller is enabled to get the use-count right.

Cc: <stable@vger.kernel.org> # v5.10+
Fixes: 09c4381 ("drm/vc4: hdmi: Implement finer-grained hooks")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Due to a FIFO that cannot be flushed between the pixelvalve and the HDMI
controllers on BCM2711, we need to carefully disable both at boot time
if they were left enabled by the firmware.

However, at the time we're running that code, the struct drm_connector
encoder pointer isn't set yet, and thus we cannot retrieve the encoder
associated to our CRTC.

We can however make use of the fact that we have a less flexible setup
than what DRM allows where we have a 1:1 relationship between our CRTCs
and encoders (and connectors), and thus store the crtc associated to our
encoder at boot time.

We cannot do that at the time the encoders are probed though, since the
CRTCs won't be probed yet and thus we don't know at that time which CRTC
index we're going to get, so let's do this in two passes: we can first
bind all the components and then once they all are bound, we can iterate
over all the encoders to find their associated CRTC and set the pointer.

This is similar to what we're doing to set the possible_crtcs field.

Fixes: 875a4d5 ("drm/vc4: drv: Disable the CRTC at boot time")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
We're going to need to tell whether we want to run with a full or
limited range RGB output in multiple places in the code, so let's create
a helper that will return whether we need with full range or not.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
The CSC callbacks takes a boolean as an argument to tell whether we're
using the full range or limited range RGB.

However, with the upcoming YUV support, the logic will be a bit more
complex. In order to address this, let's make the callbacks take the
entire mode, and call our new helper to tell whether the full or limited
range RGB should be used.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
The limited_rgb_range field in the vc4_hdmi_encoder structure is used to
tell whether we're supposed to output with a full or limited RGB range.

This is redundant with the new helper we introduced, so let's convert to
that helper and drop that field.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Converting the HDMI controller to a bridge seems like the preferred way
to support an YUV output, so let's do this.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
On the BCM2711, the HDMI_VEC_INTERFACE_XBAR register configuration
depends on whether we're using an RGB or YUV output. Let's move that
configuration to the CSC setup.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
On BCM2711, the HDMI_CSC_CTL register value has been hardcoded to an
opaque value. Let's replace it with properly defined values.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
The current CSC setup code for the BCM2711 uses a sequence of register
writes to configure the CSC depending on whether we output using a full
or limited range.

However, with the upcoming introduction of the YUV output, we're going
to add new matrices to perform the conversions, so we should switch to
something a bit more flexible that takes the matrix as an argument and
programs the CSC accordingly.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
In order to support the YUV output, we'll need the atomic state to know
what is the state of the associated property in the CSC setup callback.

Let's change the prototype of that callback to allow us to access it.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
In order to support a YUV output, we're going to need to have access to
the bridge state in the vc4_hdmi_set_avi_infoframe function. Since we
also need the connector state in that function, let's pass the full
atomic state.

While we're at it, since all those functions actually need the vc4_hdmi
structure, let's pass it instead of the drm_encoder.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
The HDMI controllers in the BCM2711 support YUV444 and YUV420 outputs,
let's add support for it.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
In order to implement a fallback mechanism to YUV422 when the pixel rate
is too high, let's move the pixel rate computation to a function of its
own that will be shared across two functions.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
When using the modes that need the highest pixel rate we support (such
as 4k at 60Hz), using a 10 or 12 bpc output will put us over the limit
of what we can achieve.

In such a case, let's force our output to be YUV422 so that we can go
back down under the required clock rate.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
@mripard
Copy link
Copy Markdown
Contributor Author

mripard commented Mar 25, 2022

This work has been superseded by PR #4754, which has been merged in the RPi tree and upstream. Closing.

@mripard mripard closed this Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant